feat: Add std.parseCsv and std.manifestCsv#701
feat: Add std.parseCsv and std.manifestCsv#701rohitjangid wants to merge 3 commits intogoogle:masterfrom
Conversation
rohitjangid
changed the title
builtins.go
Outdated
| } | ||
|
|
||
| if row == 0 { // consider first row as header | ||
| keys = record |
There was a problem hiding this comment.
I suppose we might make this a user option in future... No need to do it now though.
builtins.go
Outdated
| for _, k := range record { | ||
| keyCount[k]++ | ||
| if c := keyCount[k]; c > 1 { | ||
| keys = append(keys, fmt.Sprintf("%s__%d", k, c-1)) |
There was a problem hiding this comment.
I think this is actually going to be confusing because it's not going to round-trip back through the manifest function (the mangled column names will appear in the output). Also the behaviour would have to be documented which would be a pain. Now you've renamed the function and it doesn't look like a generic CSV reader anymore I think what you were originally doing was fine.
There was a problem hiding this comment.
I guess we can add an argument called handle_duplicate_headers=false for users to opt for this. Using this flag, we could either overwrite the duplicate headers or rename the duplicate fields as above. WDYT?
|
Hi @sparkprime |
|
@sparkprime nudging this PR |
3 participants
Implement
std.parseCsvandstd.manifestCsvin standard librarycpp-jsonnet PR: google/jsonnet#1088